Skip to content

feat(trace): support runtime sampler updates - #8716

Closed
abhiramaab wants to merge 3 commits into
open-telemetry:mainfrom
abhiramaab:feat/runtime-sampler-config
Closed

feat(trace): support runtime sampler updates#8716
abhiramaab wants to merge 3 commits into
open-telemetry:mainfrom
abhiramaab:feat/runtime-sampler-config

Conversation

@abhiramaab

Copy link
Copy Markdown

Summary

Add runtime sampler updates to SdkTracerProvider so a sampler can be changed after the provider is built.

Changes

  • Add SdkTracerProvider.setSampler(Sampler)
  • Make the shared sampler reference mutable so new spans observe updates
  • Add tests for:
    • updating the sampler at runtime
    • null rejection
    • an existing tracer observing the new sampler for new spans

Verification

  • ./gradlew :sdk:trace:test --tests io.opentelemetry.sdk.trace.SdkTracerProviderTest
  • ./gradlew :sdk:trace:test
  • ./gradlew spotlessApply
  • git diff --check

Copilot AI lite review requested due to automatic review settings August 10, 2026 11:09
@abhiramaab
abhiramaab requested a review from a team as a code owner August 10, 2026 11:09
@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 10, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: abhiramaab / name: abhiramaab (213ffb5)

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 10, 2026

Copy link
Copy Markdown

Pull request dashboard status

Closed · refreshed 2026-08-18 03:16 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for updating the SdkTracerProvider sampler at runtime so that new spans (including those created from previously obtained Tracer instances) observe the updated sampler.

Changes:

  • Introduces SdkTracerProvider.setSampler(Sampler) and corresponding test coverage for runtime updates and null rejection.
  • Makes the shared sampler reference mutable (volatile) so new span creation uses the latest sampler.
  • Adds a test verifying sampler updates affect spans started from an existing Tracer.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java Adds public runtime sampler setter and keeps sampler accessible via getSampler().
sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java Makes sampler mutable to allow runtime updates for span creation.
sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java Adds tests for sampler updates, null rejection, and existing tracer behavior.
Suppressed comments (1)

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java:125

  • getSampler() now returns the current sampler (it can change at runtime), but its Javadoc still says "configured" which reads like an immutable build-time value.
  /** Returns the configured {@link Sampler}. */
  public Sampler getSampler() {
    return sharedState.getSampler();
  }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +82 to +84
void setSampler(Sampler sampler) {
this.sampler = sampler;
}
Comment on lines +113 to +117
/**
* Updates the sampler used for new spans.
*
* @param sampler the sampler to use for sampling new spans.
*/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up: added sampler lifecycle handling so replaced samplers are shut down, and samplers set after provider shutdown are immediately shut down. Added tests covering both cases. :sdk:trace:test passes.

@opentelemetry-pr-dashboard

This comment has been minimized.

@breedx-splk breedx-splk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure that this is NOT desired behavior. I don't think users generally want a given tracer's sampler (or in this case, all tracers from a tracer provider) to be swapped out at any time at runtime. We try and keep the core of the sdk largely immutable, and this is a design principle.

Imagine that you have production user code that sets a sampler, and then some library code comes along and changes it to something entirely different, long after the sdk is created. That just makes things completely difficult to reason about or troubleshoot when the expected thing eventually happens.

Can you elaborate on what your use case is? Is it something that you can accomplish by building a mutable sampler instead for your purposes, rather than mutating the tracer provider?

@breedx-splk breedx-splk added the needs author feedback Waiting for additional feedback from the author label Aug 18, 2026
@abhiramaab

Copy link
Copy Markdown
Author

hanks for the feedback. I don't have a concrete use case that requires replacing the sampler on SdkTracerProvider after construction. The intent of this PR was to provide runtime sampler configuration, but I agree that mutating the provider's sampler conflicts with the SDK's immutability design and can make behavior difficult to reason about. A mutable Sampler would be a better fit for cases that require changing sampling behavior at runtime. I'll close this PR rather than introduce an API that isn't justified by a concrete use case.

@abhiramaab abhiramaab closed this Aug 18, 2026
@github-actions github-actions Bot removed the needs author feedback Waiting for additional feedback from the author label Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants